Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

Review follow-up (pushed via maintainer edit — @cpegeric please take a look):

Assessment of the three CHANGES_REQUESTED reviews against the head:

• @XuPeng-SH's default-path finding: already addressed by the current head (unconditional quoting in FullTextMatchExpr.Format + default-path regressions in TestFullTextMatchDeparseRoundTrip/TestValid).
• @iamlinjunhong's NO_BACKSLASH_ESCAPES doubling: mechanism addressed by the head (WithNoBackslashEscape + the four requested round-trip cases), but —
• @gouhongshen's follow-up was correct: no production caller passed the option. CTAS formatted AsSource with only quote options while the generated INSERT ... SELECT is re-parsed by the internal executor under the session's sql_mode — so the doubling bug persisted in production and only the test opted in.

Fixed in e3eca1a:

• buildTable now resolves the session sql_mode (same parserSQLModeFromContext helper the view path uses) and passes WithNoBackslashEscape to the CTAS formatter when the mode is active.
• Regression TestCTASFullTextNoBackslashEscapes: builds the CTAS plan under NO_BACKSLASH_ESCAPES (mock sql_mode override, mode-aware parse) and asserts the generated CreateAsSelectSql keeps the literal single backslash — plus the default mode still re-escapes. Verified it fails without the build_ddl.go change and passes with it. I chose this plan-level regression over a BVT because fulltext tokenizers split on \, so a doubled pattern produces identical match results end-to-end — the BVT would be blind to exactly the bug it's meant to catch; the generated-SQL assertion is the observable contract.
• Also merged current main (was DIRTY): kept both FmtCtx additions (noBackslashEscape + main's paramExprOffset) and both test insertions.

One known adjacent gap, deliberately out of scope: ordinary (non-MATCH) string literals in CTAS go through FmtCtx.WriteValue's %q path, which also backslash-escapes regardless of mode — a pre-existing issue affecting all quoted formatting, not introduced here. Happy to file it separately.

Filed the adjacent WriteValue %q gap as #26300, with an empirical probe of the three failure classes (backslash doubling under NO_BACKSLASH_ESCAPES, outright syntax errors for double-quote-containing literals under NBE/ANSI_QUOTES, and silent control-char drift) plus the list of affected WithQuoteString production callers. Out of scope here as discussed.

@gouhongshen's P1 is correct — thanks, the automated review caught a wrong premise in my previous commit. Verified all three links: the CTAS follow-up INSERT runs through runSqlWithResultAndOptions → internal SQLExecutor, whose Exec calls parsers.Parse (pkg/sql/compile/sql_executor.go:351) with an empty SQL mode, and ParseSQLModeFlags("") leaves backslash-escape processing ON in the scanner. So the reparse is always default-mode: under an NBE session my conditional emitted a single backslash that the executor's parse consumed as an escape (a\nb → a<newline>b), while the original unconditional doubling round-tripped correctly. My earlier regression asserted the generated SQL text instead of the reparse result — the wrong contract.

Fixed in 2224671 (reviewer's option A — keep CTAS formatting compatible with the enforced default parser):

• Reverted the WithNoBackslashEscape wiring in buildTable; the comment there now states the real contract (CTAS literals must stay default-escaped because the internal executor always parses in default mode — do not make this session-mode-aware unless the executor's parse becomes so too). The formatter option itself stays: it is still correct for same-mode round-trips and keeps the parser-level TestFullTextMatchPatternRoundTrip cases valid.
• Replaced the test with TestCTASFullTextPatternSurvivesInternalReparse, which asserts the requested contract: build the CTAS plan under a session sql_mode, reparse CreateAsSelectSql exactly as the executor does (default mode), and require the resulting MATCH pattern to equal what the user wrote — for both NO_BACKSLASH_ESCAPES and default sessions. Verified it fails against the previous conditional and passes with this change; related parser/plan tests and vet are green.

I chose option A over propagating session sql_mode into the internal parser because that would change parsing for all internal SQL (much of it system-generated assuming default escaping) — too risky for this PR's scope.

@cpegeric FYI — pushed via maintainer edit again.

@Mergifyio queue

Merge Queue Status

• 🟠 Waiting for queue conditions
• ⏳ Enter queue
• ⏳ Run checks
• ⏳ Merge

@iamlinjunhong could you take another look? Your NO_BACKSLASH_ESCAPES round-trip concern was addressed (WithNoBackslashEscape formatter option + the four requested round-trip cases in TestFullTextMatchPatternRoundTrip), and the follow-up review iterations settled the CTAS wiring: the internal executor always reparses in default mode, so CTAS formatting stays default-escaped with TestCTASFullTextPatternSurvivesInternalReparse asserting the reparse contract. gouhongshen and XuPeng-SH have both approved the current head; your CHANGES_REQUESTED is the last gate before the merge queue.